sdk/runanywhere-commons/src/utils/rac_image_utils.cpp-316-318 (1)

316-318: ⚠️ Potential issue | 🟡 Minor

Good validation, but inconsistent error code.

The channel validation correctly prevents out-of-bounds access to the mean/std arrays (size 3). However, other validation checks in this file use RAC_ERROR_INVALID_ARGUMENT (e.g., lines 251, 396, 426), while this uses RAC_ERROR_INVALID_PARAMETER.

Proposed fix for consistency

     if (image->channels < 1 || image->channels > 3) {
-        return RAC_ERROR_INVALID_PARAMETER;
+        return RAC_ERROR_INVALID_ARGUMENT;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/utils/rac_image_utils.cpp` around lines 316 -
318, The channel validation returns RAC_ERROR_INVALID_PARAMETER but other
validations in this file use RAC_ERROR_INVALID_ARGUMENT; update the check that
inspects image->channels (the if (image->channels < 1 || image->channels > 3)
branch) to return RAC_ERROR_INVALID_ARGUMENT instead of
RAC_ERROR_INVALID_PARAMETER so error codes are consistent across the file.

sdk/runanywhere-commons/src/infrastructure/network/api_types.cpp-533-539 (1)

533-539: ⚠️ Potential issue | 🟡 Minor

Add explicit include for SIZE_MAX definition.

The overflow check correctly guards against size_t wraparound: the logic event_count > (SIZE_MAX - kBaseEstimate) / kPerEventEstimate ensures the subsequent multiplication and addition won't overflow.

However, SIZE_MAX is not explicitly included. It's defined in <climits> or <cstdint>, neither of which appears in this file's includes. While some implementations transitively include it through <cstdlib>, this is not guaranteed by the C++ standard—particularly important given that this project explicitly uses gcc/g++ and notes that clang lacks proper C++ stdlib header support on the target platform. Per the Google C++ Style Guide followed by this project, add an explicit include:

`#include` <climits>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/infrastructure/network/api_types.cpp` around
lines 533 - 539, Add an explicit include for the header that defines SIZE_MAX by
inserting `#include` <climits> among the other includes at the top of
api_types.cpp so the overflow check that uses SIZE_MAX (around the calculation
with kPerEventEstimate, kBaseEstimate, and batch->event_count) is well-defined
across toolchains; ensure the new include is placed with the existing system
headers.

sdk/runanywhere-commons/src/features/llm/llm_component.cpp-706-712 (1)

706-712: ⚠️ Potential issue | 🟡 Minor

Emit a terminal failure event on this OOM return.

This branch returns before rac_lifecycle_track_error() and RAC_EVENT_LLM_GENERATION_FAILED, so a started stream can disappear from telemetry if the final strdup() fails.

Suggested patch

     final_result.text = strdup(ctx.full_text.c_str());
     if (!final_result.text) {
         log_error("LLM.Component", "Failed to allocate result text");
+        rac_lifecycle_track_error(component->lifecycle, RAC_ERROR_OUT_OF_MEMORY,
+                                  "generateStream");
+
+        rac_analytics_event_data_t event = {};
+        event.type = RAC_EVENT_LLM_GENERATION_FAILED;
+        event.data.llm_generation = RAC_ANALYTICS_LLM_GENERATION_DEFAULT;
+        event.data.llm_generation.generation_id = generation_id.c_str();
+        event.data.llm_generation.model_id = model_id;
+        event.data.llm_generation.model_name = model_name;
+        event.data.llm_generation.error_code = RAC_ERROR_OUT_OF_MEMORY;
+        event.data.llm_generation.error_message = "Failed to allocate result text";
+        rac_analytics_event_emit(RAC_EVENT_LLM_GENERATION_FAILED, &event);
+
         if (error_callback) {
             error_callback(RAC_ERROR_OUT_OF_MEMORY, "Failed to allocate result text", user_data);
         }
         return RAC_ERROR_OUT_OF_MEMORY;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/features/llm/llm_component.cpp` around lines 706
- 712, The OOM branch that returns when final_result.text is null should also
emit the terminal failure to telemetry: before returning from the allocation
check in the LLM component, call rac_lifecycle_track_error(...) with the
appropriate context and error code and emit RAC_EVENT_LLM_GENERATION_FAILED (via
the existing event tracking function used elsewhere) so the started stream is
marked failed; keep the existing log_error and error_callback but add the
rac_lifecycle_track_error(...) and the event emit (including any
message/user_data) immediately prior to returning RAC_ERROR_OUT_OF_MEMORY.

sdk/runanywhere-commons/src/features/stt/stt_component.cpp-342-345 (1)

342-345: ⚠️ Potential issue | 🟡 Minor

Use the resolved request sample_rate in telemetry.

local_options is the effective per-call config, but sample_rate is copied from component->config.sample_rate. If callers override options->sample_rate, the started/completed events will report the default instead of the value actually used for transcription.

🔧 Minimal fix

-        sample_rate = component->config.sample_rate;
-
         // Copy effective options to local so we can release the lock
         local_options = options ? *options : component->default_options;
+        sample_rate = local_options.sample_rate;

Also applies to: 382-385, 441-443

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/features/stt/stt_component.cpp` around lines 342
- 345, Telemetry uses component->config.sample_rate instead of the per-call
resolved value; after copying the effective per-call config into local_options
(from options ? *options : component->default_options), assign sample_rate =
local_options.sample_rate and use that for the started/completed events so
overridden options->sample_rate is reported; apply the same change in the other
occurrences where sample_rate is taken from component->config (the other blocks
around the start/completed event emission).

sdk/runanywhere-commons/src/features/stt/stt_component.cpp-16-16 (1)

16-16: ⚠️ Potential issue | 🟡 Minor

Add the missing header for snprintf.

Line 70 uses snprintf, but <cstdio> is not included. The code currently relies on transitive declarations, which is not portable. Add <cstdio> to the includes and preferably use std::snprintf for C++17 compliance.

Suggested fix

 `#include` <chrono>
+#include <cstdio>
 `#include` <cstdlib>
 `#include` <cstring>
 `#include` <mutex>
 `#include` <random>

Then at line 70:

-    snprintf(buffer, sizeof(buffer), "trans_%08x%08x", dis(gen), dis(gen));
+    std::snprintf(buffer, sizeof(buffer), "trans_%08x%08x", dis(gen), dis(gen));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/features/stt/stt_component.cpp` at line 16, Add
the missing C stdio header and prefer the C++-qualified function: include
<cstdio> alongside the existing includes (the file currently only shows `#include`
<random>) and replace or call the function as std::snprintf where snprintf is
used (the call around line 70) so the code no longer relies on transitive
declarations and is C++17-compliant.

sdk/runanywhere-kotlin/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/foundation/bridge/CppBridge.kt-211-212 (1)

211-212: ⚠️ Potential issue | 🟡 Minor

Add CppBridgeFileManager.unregister() and call it during shutdown for consistency.

CppBridgeFileManager.register() registers JNI file I/O callbacks, but there is no corresponding unregister() method or cleanup in shutdown(). Every other bridge extension follows a symmetric register/unregister lifecycle. Add an unregister() method to CppBridgeFileManager and call it in CppBridge.shutdown() to ensure proper cleanup on shutdown cycles and match the established pattern.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-kotlin/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/foundation/bridge/CppBridge.kt`
around lines 211 - 212, Add a symmetric cleanup path for the file manager:
implement CppBridgeFileManager.unregister() to undo whatever
CppBridgeFileManager.register() sets up (remove JNI callbacks/resources) and
invoke CppBridgeFileManager.unregister() from CppBridge.shutdown() (alongside
other bridge unregister calls) so the CppBridge.shutdown() lifecycle mirrors
registration and performs proper cleanup.

sdk/runanywhere-kotlin/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/foundation/bridge/extensions/CppBridgeFileManager.kt-115-123 (1)

115-123: ⚠️ Potential issue | 🟡 Minor

Only treat an existing directory as success.

dir.exists() is also true for a regular file, so this reports success even when the target path is unusable as a directory. Check isDirectory before returning success.

Suggested fix

-                if (success || dir.exists()) 0 else -180 // RAC_ERROR_DIRECTORY_CREATION_FAILED
+                if (success || dir.isDirectory) 0 else -180 // RAC_ERROR_DIRECTORY_CREATION_FAILED

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-kotlin/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/foundation/bridge/extensions/CppBridgeFileManager.kt`
around lines 115 - 123, In createDirectory(path: String, recursive: Boolean)
update the success condition so an existing regular file isn't treated as a
successful directory creation: after attempting dir.mkdirs()/dir.mkdir() only
return success (0) if the operation succeeded or if dir.isDirectory() is true;
otherwise return the error code -180. Ensure the catch block still returns -180
and reference the createDirectory function and the local variable dir when
making this change.

sdk/runanywhere-swift/Sources/RunAnywhere/Infrastructure/Download/Services/ExtractionService.swift-149-153 (1)

149-153: ⚠️ Potential issue | 🟡 Minor

Don't hard-code fileCount to zero.

Every successful extraction now reports fileCount: 0, which makes ExtractionResult internally inconsistent and drops extraction stats for any downstream UI or analytics.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Infrastructure/Download/Services/ExtractionService.swift`
around lines 149 - 153, The return currently hard-codes fileCount: 0 in the
ExtractionResult creating an incorrect metric; change it to return the actual
number of files produced by the extraction (e.g. use the count of the extracted
files collection or compute files under modelPath) so
ExtractionResult(modelPath: modelPath, extractedSize: extractedSize, fileCount:
<actualCount>, durationSeconds: duration) reflects the real value—look for the
extraction routine's list/array (e.g. extractedFiles, filesExtracted, or use
FileManager.contentsOfDirectory(atPath:) on modelPath) and substitute that count
for fileCount.

sdk/runanywhere-commons/src/infrastructure/download/download_orchestrator.cpp-171-174 (1)

171-174: ⚠️ Potential issue | 🟡 Minor

This depth guard skips the last level.

With depth >= max_depth, a search starting at depth = 0 never inspects level max_depth itself. The current max_depth = 2 call only visits depths 0 and 1.

Suggested fix

-    if (depth >= max_depth) return false;
+    if (depth > max_depth) return false;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-commons/src/infrastructure/download/download_orchestrator.cpp`
around lines 171 - 174, The depth check in find_single_model_file incorrectly
stops when depth == max_depth so the final level is never searched; change the
guard from "if (depth >= max_depth) return false;" to "if (depth > max_depth)
return false;" (and ensure any recursive calls pass depth+1 so the function can
execute at depth == max_depth) so the function will inspect the max_depth level
as intended; reference: function find_single_model_file, parameters depth and
max_depth, and output buffer out_path.

sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp-4306-4314 (1)

4306-4314: ⚠️ Potential issue | 🟡 Minor

Missing exception check after NewStringUTF.

If NewStringUTF fails (OOM), it returns nullptr and throws a pending exception. Calling JNI methods with a pending exception causes undefined behavior.

🛡️ Proposed fix

 static rac_result_t jni_fc_create_directory(const char* path, int recursive, void* user_data) {
     JNIEnv* env = getJNIEnv();
     if (env == nullptr || g_file_callbacks_obj == nullptr) return RAC_ERROR_NOT_INITIALIZED;
     jstring jPath = env->NewStringUTF(path);
+    if (jPath == nullptr) {
+        if (env->ExceptionCheck()) env->ExceptionClear();
+        return RAC_ERROR_OUT_OF_MEMORY;
+    }
     jint result = env->CallIntMethod(g_file_callbacks_obj, g_fc_create_directory, jPath,
                                      static_cast<jboolean>(recursive != 0));
     env->DeleteLocalRef(jPath);
     return static_cast<rac_result_t>(result);
 }

The same pattern should be applied to jni_fc_delete_path, jni_fc_list_directory, jni_fc_path_exists, and jni_fc_get_file_size.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp` around lines
4306 - 4314, The NewStringUTF call in jni_fc_create_directory can return nullptr
and leave a pending exception (OOM); update jni_fc_create_directory to check if
jPath is nullptr or env->ExceptionCheck() is true after NewStringUTF, clean up
any local refs if needed, clear or leave the exception handled appropriately and
return RAC_ERROR_NOT_INITIALIZED (or an appropriate rac_result_t) instead of
calling CallIntMethod; apply the same null/exception-check pattern to
jni_fc_delete_path, jni_fc_list_directory, jni_fc_path_exists, and
jni_fc_get_file_size so none call JNI methods when NewStringUTF failed.

sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp-4427-4454 (1)

4427-4454: ⚠️ Potential issue | 🟡 Minor

Missing verification that all method IDs were found.

If any GetMethodID call fails (method doesn't exist on the Kotlin side), it returns nullptr and sets a pending exception. The code should verify all method IDs before returning success.

🛡️ Proposed fix to verify method IDs

     g_fc_get_available_space = env->GetMethodID(cls, "getAvailableSpace", "()J");
     g_fc_get_total_space = env->GetMethodID(cls, "getTotalSpace", "()J");
     env->DeleteLocalRef(cls);

+    // Verify all required methods were found
+    if (!g_fc_create_directory || !g_fc_delete_path || !g_fc_list_directory ||
+        !g_fc_path_exists || !g_fc_is_directory || !g_fc_get_file_size ||
+        !g_fc_get_available_space || !g_fc_get_total_space) {
+        LOGe("File manager callbacks: missing required methods");
+        if (env->ExceptionCheck()) {
+            env->ExceptionDescribe();
+            env->ExceptionClear();
+        }
+        env->DeleteGlobalRef(g_file_callbacks_obj);
+        g_file_callbacks_obj = nullptr;
+        return RAC_ERROR_INVALID_ARGUMENT;
+    }
+
     LOGi("File manager callbacks registered");
     return RAC_SUCCESS;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp` around lines
4427 - 4454, Verify that every cached jmethodID (g_fc_create_directory,
g_fc_delete_path, g_fc_list_directory, g_fc_path_exists, g_fc_is_directory,
g_fc_get_file_size, g_fc_get_available_space, g_fc_get_total_space) is non-null
after the GetMethodID calls in nativeFileManagerRegisterCallbacks; if any are
null, clean up by deleting the global ref g_file_callbacks_obj (and set it to
nullptr), delete the local class ref, check/clear the pending JNI exception via
env->ExceptionCheck()/env->ExceptionClear(), and return an appropriate error
code (e.g., RAC_ERROR_METHOD_NOT_FOUND) instead of RAC_SUCCESS. Ensure normal
flow returns RAC_SUCCESS only when all method IDs are valid.

sdk/runanywhere-swift/Sources/RunAnywhere/Infrastructure/FileManagement/Services/SimplifiedFileManager.swift (1)

121-124: Potential main-thread blocking and code duplication.

1. @mainactor with I/O: This method calls the C++ bridge for file system checks while constrained to the main thread, which could cause UI hitches if the operation takes time.

2. Duplication: This method is identical to modelFolderExists(modelId:framework:) at line 61. Consider having one call the other or extracting to a single implementation.

♻️ Suggested refactor

     `@MainActor`
     public func isModelDownloaded(modelId: String, framework: InferenceFramework) -> Bool {
-        return CppBridge.FileManager.modelFolderHasContents(modelId: modelId, framework: framework)
+        return modelFolderExists(modelId: modelId, framework: framework)
     }

Also consider whether @MainActor is necessary here, or if this could be made nonisolated or async to avoid blocking the main thread.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Infrastructure/FileManagement/Services/SimplifiedFileManager.swift`
around lines 121 - 124, The isModelDownloaded(modelId:framework:) method
duplicates modelFolderExists(modelId:framework:) and is marked `@MainActor` while
calling a potentially blocking C++ file-system check; change isModelDownloaded
to reuse the existing implementation by delegating to
modelFolderExists(modelId:framework:) (or extract a shared private helper that
calls CppBridge.FileManager.modelFolderHasContents) and remove the `@MainActor`
isolation so the call won’t block the main thread—alternatively make it
async/nonisolated if the surrounding API expects concurrency; ensure any shared
helper has appropriate thread-safety if necessary.

sdk/runanywhere-commons/src/utils/rac_image_utils.cpp (1)

493-497: Reasonable defensive fix; consider logging a warning.

Clamping invalid dimensions to 1x1 prevents division by zero in the aspect ratio calculation below. However, silently fixing invalid input may mask bugs in upstream code. A debug log would help diagnose issues without changing behavior.

Optional: Add debug logging for invalid dimensions

     if (width <= 0 || height <= 0) {
+        RAC_LOG_DEBUG(LOG_CAT, "Invalid dimensions (%dx%d), clamping to 1x1", width, height);
         *out_width = 1;
         *out_height = 1;
         return;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/utils/rac_image_utils.cpp` around lines 493 -
497, When detecting invalid input in the width/height check (the if (width <= 0
|| height <= 0) block that sets *out_width and *out_height to 1), add a
debug/warning log before clamping so upstream issues are visible; emit a concise
message that includes the offending width and height values and the
function/variable context (width, height, out_width, out_height) using the
project’s existing logger API (e.g., LOG_WARN/LOG_DEBUG or the module’s logger)
while keeping the current behavior of setting outputs to 1 and returning.

sdk/runanywhere-swift/Sources/RunAnywhere/CRACommons/include/rac_file_manager.h (1)

287-299: Don't publish an "internal helper" as RAC_API.

Line 298 exports rac_file_manager_clear_directory from the public header while the section/doc call it internal. That hardens a private helper into supported API surface for every SDK consumer. Either move it to a private header or document it as a stable public primitive.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-swift/Sources/RunAnywhere/CRACommons/include/rac_file_manager.h`
around lines 287 - 299, The declaration rac_file_manager_clear_directory is
marked with RAC_API in the public header but is described as an internal helper;
remove it from the public API by either (A) moving the declaration (and any
related types like rac_file_callbacks_t usage) into a private/internal header or
compilation unit and removing the RAC_API annotation from the public header, or
(B) if it must remain public, update the header comment and documentation to
declare rac_file_manager_clear_directory as a supported public primitive and
keep RAC_API; choose one approach and apply consistently to avoid exporting an
unintended private symbol.

sdk/runanywhere-flutter/packages/runanywhere/lib/native/ffi_types.dart (1)

1473-1484: Document memory ownership for recommendation pointer.

RacStorageAvailabilityStruct.recommendation is a Pointer<Utf8> that callers must handle correctly. Consider adding a doc comment clarifying whether this string is owned by the struct (and freed with it) or must be freed separately by the caller.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-flutter/packages/runanywhere/lib/native/ffi_types.dart`
around lines 1473 - 1484, RacStorageAvailabilityStruct.recommendation is a raw
Pointer<Utf8> with no documented ownership/lifetime — add a doc comment on the
recommendation field (and near the RacStorageAvailabilityStruct declaration)
that states whether the pointer is owned by the struct or by the caller, whether
the string is null-terminated, how long the pointer is valid (e.g. only until
the struct is freed or until an explicit free), and the exact method to release
it if the caller must free it (e.g. call freeString(), free via malloc.free, or
use a provided API function); reference
RacStorageAvailabilityStruct.recommendation in the comment so callers know to
follow the specified freeing convention.

sdk/runanywhere-commons/src/infrastructure/network/auth_manager.cpp (1)

301-302: Good security practice clearing sensitive data from stack.

Zeroing the buffer after loading tokens helps prevent sensitive credential leakage via stack inspection. Note that aggressive compiler optimizations may elide this memset if the buffer is not subsequently used. For stronger guarantees, consider using explicit_bzero (POSIX) or SecureZeroMemory (Windows) if available.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/infrastructure/network/auth_manager.cpp` around
lines 301 - 302, The current memset(buffer, 0, sizeof(buffer)) can be optimized
away by compilers; replace this with a platform-appropriate secure zero routine
to ensure memory is wiped (e.g., call explicit_bzero(buffer, sizeof(buffer)) on
POSIX, SecureZeroMemory(buffer, sizeof(buffer)) on Windows, or a fallback to a
volatile memset wrapper if neither is available), keeping the same buffer
identifier so the sensitive token storage is cleared reliably; update any
`#includes` and add conditional compilation helpers (or a small secure_zero(void*,
size_t) wrapper) and use that in place of the existing memset call.

sdk/runanywhere-commons/tests/test_download_orchestrator.cpp (1)

42-50: Prefer std::filesystem over shelling out in these helpers.

system("rm -rf ...") / system("mkdir -p ...") makes the tests depend on /bin/sh and still leaves quoting edge cases. C++17 already gives you remove_all and create_directories in-process.

Possible cleanup

Add #include <filesystem> near the other headers, then:

 static void remove_dir(const std::string& path) {
-    std::string cmd = "rm -rf \"" + path + "\"";
-    system(cmd.c_str());
+    std::error_code ec;
+    std::filesystem::remove_all(path, ec);
 }

 static void mkdir_p(const std::string& path) {
-    std::string cmd = "mkdir -p \"" + path + "\"";
-    system(cmd.c_str());
+    std::error_code ec;
+    std::filesystem::create_directories(path, ec);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/tests/test_download_orchestrator.cpp` around lines 42
- 50, Replace shelling out in the helpers by using std::filesystem: include
<filesystem>, change remove_dir to call std::filesystem::remove_all(path) and
change mkdir_p to call std::filesystem::create_directories(path); ensure you
handle/propagate errors or ignore return values consistently (remove_all returns
count, create_directories returns bool) and remove the system(...) calls in
remove_dir and mkdir_p.

sdk/runanywhere-commons/include/rac/infrastructure/extraction/rac_extraction.h (1)

82-135: Document the callback/lifetime contract in the public header.

This API documents parameters and error codes, but not whether rac_extraction_progress_fn is synchronous, which thread invokes it, or how long user_data and out_result must remain valid. That lifecycle contract belongs in the header for SDK integrators. As per coding guidelines, "Public C API headers in include/rac/ must document vtable operations, error codes, and lifecycle requirements."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-commons/include/rac/infrastructure/extraction/rac_extraction.h`
around lines 82 - 135, The header is missing the lifecycle and threading
contract for rac_extraction_progress_fn and the pointer parameters; update the
comments for rac_extraction_progress_fn and rac_extract_archive_native to state
explicitly whether the callback is invoked synchronously or on a separate
thread, whether it may be re-entered or called concurrently, and the valid
lifetimes for user_data and out_result. Specifically: document that
rac_extraction_progress_fn is invoked by rac_extract_archive_native during the
extraction operation (synchronously on the extraction thread/caller thread),
that the callback will not be called after rac_extract_archive_native returns,
and that the caller must keep user_data and the out_result storage valid for the
entire duration of rac_extract_archive_native; also note thread-safety
expectations (e.g., callback may be called serially, not concurrently) and
mention behavior if async/streaming options change (point to
rac_extraction_options_t if it can enable async behavior). Include these
lifecycle sentences in the public header adjacent to the typedef for
rac_extraction_progress_fn and the rac_extract_archive_native declaration so
integrators know the contract.

sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp (1)

4294-4304: File callbacks globals lack thread-safety protection.

The global variables g_file_callbacks_obj and cached method IDs are accessed without mutex protection. If nativeFileManagerRegisterCallbacks is called while a file operation is in progress on another thread, this could cause a race condition.

Consider adding a mutex (similar to g_adapter_mutex used elsewhere in this file) if concurrent registration is possible.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp` around lines
4294 - 4304, The globals for file callbacks (g_file_callbacks_obj,
g_fc_create_directory, g_fc_delete_path, g_fc_list_directory, g_fc_path_exists,
g_fc_is_directory, g_fc_get_file_size, g_fc_get_available_space,
g_fc_get_total_space) need mutex protection to avoid races during registration
and use; add a dedicated mutex (or reuse g_adapter_mutex) and lock it around
writes in nativeFileManagerRegisterCallbacks and around reads/uses in any file
operation functions that access these globals, ensure proper JNI local/global
reference handling when updating g_file_callbacks_obj while holding the lock,
and release/clear the global safely when unregistering.

sdk/runanywhere-commons/include/rac/infrastructure/file_management/rac_file_manager.h (1)

38-115: Document thread-safety guarantees and callback lifecycle requirements.

Per coding guidelines, public C API headers in include/rac/ must document lifecycle requirements. The rac_file_callbacks_t struct should clarify:

1. Whether callbacks must remain valid for the duration of any API call using them
2. Thread-safety requirements (can the same callbacks struct be used from multiple threads?)
3. Whether user_data ownership is transferred or borrowed

Consider adding a brief note, e.g.:

/**
 * `@note` Callbacks must remain valid for the duration of any rac_file_manager_* call.
 * `@note` Thread safety: Callers must synchronize concurrent access to shared user_data.
 */

Based on learnings: "Public C API headers in include/rac/ must document vtable operations, error codes, and lifecycle requirements".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-commons/include/rac/infrastructure/file_management/rac_file_manager.h`
around lines 38 - 115, Add lifecycle and thread-safety notes to the
rac_file_callbacks_t declaration: state that callbacks in rac_file_callbacks_t
must remain valid for the duration of any API call that receives the struct
(e.g., rac_file_manager_* functions), clarify that user_data is borrowed
(ownership not transferred) unless otherwise documented, and require callers to
synchronize concurrent access if the same rac_file_callbacks_t (or its
user_data) is used from multiple threads; reference the struct name
rac_file_callbacks_t and the user_data field in the note so readers can find the
relevant vtable and understand callback lifetime and thread-safety constraints.

sdk/runanywhere-commons/src/features/voice_agent/voice_agent.cpp (1)

240-261: ⚠️ Potential issue | 🔴 Critical

rac_voice_agent_destroy() still tears down the handle while callers can be live.

The mutex only protects threads that are already inside the critical section. Threads still queued on handle->mutex can wake after the lock drops, and rac_voice_agent_detect_speech() still bypasses that mutex entirely, so Line 261 can free the handle while other APIs still dereference it. Add a real shutdown barrier/in-flight counter and wait for it to drain before deleting the object.

As per coding guidelines "Voice agent implementation must support configuration of individual components (LLM, STT, TTS, VAD) and thread-safe pipeline orchestration".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/features/voice_agent/voice_agent.cpp` around
lines 240 - 261, rac_voice_agent_destroy() currently releases handle->mutex and
immediately deletes handle while callers (e.g., rac_voice_agent_detect_speech)
may still access it; add a proper shutdown barrier by introducing an in-flight
counter and a shutdown flag on the handle (e.g., atomic<int> inflight_count and
atomic<bool> shutting_down), incrementing/decrementing inflight_count in
entry/exit paths of public APIs (including rac_voice_agent_detect_speech) and
making rac_voice_agent_destroy() set shutting_down/is_configured=false, wait
(spin or condition_variable) until inflight_count == 0, then proceed to destroy
owned components and delete handle; update rac_voice_agent_detect_speech and
other callers to check shutting_down/is_configured and return early while still
correctly decrementing the counter so destroy can safely delete the handle.

sdk/runanywhere-commons/src/infrastructure/model_management/model_assignment.cpp (1)

189-192: ⚠️ Potential issue | 🔴 Critical

Advance pos before continuing on allocation failure.

If either strdup fails, the loop continues without advancing pos, causing an infinite retry of the same object. This turns an OOM condition into a hang.

🐛 Proposed fix

         if (!model->id || !model->name) {
             rac_model_info_free(model);
+            pos = obj_end;
             continue;
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-commons/src/infrastructure/model_management/model_assignment.cpp`
around lines 189 - 192, The loop over model entries is not advancing pos on
allocation failure, causing an infinite retry; in the block that checks if
(!model->id || !model->name) (after strdup failures), free the model
(rac_model_info_free(model)) and then increment the loop index variable pos (or
the iterator used to traverse entries) before continuing so the code advances to
the next item instead of retrying the same one.

sdk/runanywhere-commons/src/features/vad/vad_analytics.cpp (1)

74-79: ⚠️ Potential issue | 🔴 Critical

Constructor exceptions can still cross the extern "C" boundary here.

new (std::nothrow) only converts allocation failure into nullptr; it does not swallow exceptions thrown while constructing rac_vad_analytics_s. If that constructor throws, this function still terminates the process instead of returning a rac_result_t.

In C++, does `new (std::nothrow) T(...)` suppress exceptions thrown by `T`'s constructor, or only allocation failures?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/features/vad/vad_analytics.cpp` around lines 74 -
79, The allocation can still throw from the constructor of rac_vad_analytics_s;
wrap the new (std::nothrow) rac_vad_analytics_s() call in a try/catch inside the
function (the one that sets *out_handle) so constructor exceptions are caught,
log the exception message (use exception.what()) and return a non-success
rac_result_t instead of letting the exception cross the extern "C" boundary;
also catch (...) for non-std::exception cases and return an appropriate error
code. Ensure you still handle the nullptr case (allocation failure) as before,
and reference the allocation and type name rac_vad_analytics_s and the output
pointer *out_handle in your changes.

sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp (1)

4207-4239: ⚠️ Potential issue | 🔴 Critical

Rename these JNI exports to native_bridge, not native_1bridge.

These symbols currently map to com.runanywhere.sdk.native_bridge.RunAnywhereBridge, while the bridge class in this PR lives under com.runanywhere.sdk.native.bridge.RunAnywhereBridge. The JVM will not find these methods, so every new archive/file-manager call here fails with UnsatisfiedLinkError at runtime.

#!/bin/bash
echo "=== New JNI exports ==="
rg -n "Java_com_runanywhere_sdk_native(_1)?bridge_RunAnywhereBridge_(nativeExtractArchive|nativeDetectArchiveType|nativeFindModelPathAfterExtraction|nativeDownloadRequiresExtraction|nativeComputeDownloadDestination|nativeFileManager)" sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp

echo
echo "=== Kotlin package declaration ==="
fd "RunAnywhereBridge.kt" sdk/runanywhere-kotlin -x sed -n '1,40p' {}

Also applies to: 4245-4582

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp` around lines
4207 - 4239, The JNI exported function names use "native_1bridge" but the
Java/Kotlin class lives in package com.runanywhere.sdk.native.bridge, so rename
the JNI symbols to use "native_bridge" (remove the "_1" escape) to match the
JVM-mangled names; specifically update
Java_com_runanywhere_sdk_native_1bridge_RunAnywhereBridge_nativeExtractArchive
and
Java_com_runanywhere_sdk_native_1bridge_RunAnywhereBridge_nativeDetectArchiveType
(and the other related exports
Java_com_runanywhere_sdk_native_1bridge_RunAnywhereBridge_nativeFindModelPathAfterExtraction,
_nativeDownloadRequiresExtraction, _nativeComputeDownloadDestination,
_nativeFileManager) to
Java_com_runanywhere_sdk_native_bridge_RunAnywhereBridge_<methodName> so the
signatures match com.runanywhere.sdk.native.bridge.RunAnywhereBridge and the JVM
can bind them. Ensure each renamed JNI function keeps the same parameters and
return types and update any matching forward-declarations or references in the
same file.

sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_model_paths.dart (1)

8-8: ⚠️ Potential issue | 🔴 Critical

Add the missing dart:io import.

_ensureDirectoryExists() still instantiates Directory(path), so this file will not compile until dart:io is imported.

Minimal fix

 import 'dart:ffi';
+import 'dart:io';

Run this to confirm the unresolved import:

#!/bin/bash
sed -n '1,20p' sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_model_paths.dart
echo "---"
rg -n '\bDirectory\s*\(' sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_model_paths.dart
echo "---"
rg -n "^import 'dart:io';" sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_model_paths.dart || true

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_model_paths.dart`
at line 8, The file is missing the dart:io import causing Directory usage in
_ensureDirectoryExists() to fail; add import 'dart:io'; alongside the existing
import 'package:runanywhere/native/dart_bridge_download.dart' so Directory(path)
and related types resolve and the file will compile.

sdk/runanywhere-commons/src/features/vad/vad_component.cpp (1)

184-185: ⚠️ Potential issue | 🔴 Critical

These getter locks still deadlock on callback re-entry.

rac_vad_component_process(), rac_vad_component_stop(), and rac_vad_component_cleanup() still hold component->mtx while calling rac_energy_vad_*. That path can synchronously reach vad_speech_activity_callback() and user code, so any callback that queries init/state/metrics will re-enter these getters and block on the same mutex. Use atomics/snapshots for the read APIs, or release component->mtx before calling into the VAD engine.

Also applies to: 482-503

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/features/vad/vad_component.cpp` around lines 184
- 185, The getters (eg. rac_vad_component_is_initialized) deadlock because
component->mtx is held while code paths in rac_vad_component_process,
rac_vad_component_stop, and rac_vad_component_cleanup call into rac_energy_vad_*
which can synchronously re-enter vad_speech_activity_callback and user code that
calls those getters; fix by removing long-held mutex usage: either make the
state fields atomic or take a local snapshot of the fields under the lock and
return the snapshot (no lock held during callbacks), and/or ensure
rac_vad_component_process/stop/cleanup release component->mtx before calling
rac_energy_vad_* so callbacks can safely call the getters without blocking.
Target the functions rac_vad_component_process, rac_vad_component_stop,
rac_vad_component_cleanup and the getter (is_initialized) to apply the
atomic/snapshot change or to move the rac_energy_vad_* call outside the locked
section.

sdk/runanywhere-commons/src/server/http_server.cpp (1)

57-111: ⚠️ Potential issue | 🔴 Critical

Catch exceptions before returning through rac_server_start().

std::filesystem::exists, std::make_unique, and std::thread can still throw in this startup path, and rac_server_start() forwards HttpServer::start() straight through the C API. Convert those failures to a rac_result_t instead of letting exceptions escape.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/server/http_server.cpp` around lines 57 - 111,
Wrap the body of HttpServer::start (after acquiring the mutex_) in a try/catch
that catches std::exception and ...; on exception return an appropriate
rac_result_t (e.g., RAC_ERROR_INTERNAL) and log the exception message via
RAC_LOG_ERROR; ensure any partially initialized members (server_, serverThread_,
loaded model state) are cleaned/reset before returning—target the section that
calls std::filesystem::exists, loadModel(modelPath_),
std::make_unique<httplib::Server>(), setupCors(), setupRoutes(), and
serverThread_ (std::thread(&HttpServer::serverThread, this)) so no C++ exception
propagates out of HttpServer::start and all errors convert to rac_result_t.

sdk/runanywhere-react-native/packages/core/cpp/bridges/FileManagerBridge.cpp (1)

59-86: ⚠️ Potential issue | 🔴 Critical

Don't follow symlinks during recursive deletes.

stat() resolves the target before the type check here, so a symlink inside cache/temp/model storage can make clearCache(), clearTemp(), or deleteModel() recurse outside the managed tree. Use lstat() and unlink symlinks instead of descending into them.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-react-native/packages/core/cpp/bridges/FileManagerBridge.cpp`
around lines 59 - 86, The posixDeletePath function currently uses stat which
follows symlinks and can recursively escape the managed tree; switch to lstat in
posixDeletePath to detect S_ISLNK and treat symlinks as files (call unlink on
the path and return appropriate RAC_SUCCESS/RAC_ERROR_FILE_IO) instead of
descending into them; additionally, when iterating directory entries in
posixDeletePath, call lstat on each childPath (not stat) and if S_ISLNK unlink
the child, if S_ISDIR recurse (with the same lstat-based checks), otherwise
unlink the child; ensure DIR* is closed on all paths and propagate error codes
from unlink/rmdir calls consistently.

sdk/runanywhere-commons/src/infrastructure/extraction/rac_extraction.cpp (1)

37-55: ⚠️ Potential issue | 🔴 Critical

Backslash and Windows paths still bypass the zip-slip checks.

is_path_safe() only blocks /-rooted and /../ forms, and hardlink targets are rewritten without the same validation. Entries like ..\payload, C:\payload, or \\server\share can still escape destination_dir on Windows or via link targets.

Also applies to: 217-245

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdk/runanywhere-commons/src/infrastructure/extraction/rac_extraction.cpp`
around lines 37 - 55, is_path_safe currently only checks POSIX-style absolute
and ".." components and misses Windows-style backslashes, drive letters and UNC
paths, and hardlink target validation; update is_path_safe to normalize or
reject backslashes by treating '\' as a path separator, reject Windows absolute
forms (leading drive letter like "C:" or leading "\\" for UNC), normalize
separators and collapse/sanitize "." and ".." components before acceptance, and
apply the same validation to any hardlink target rewriting logic (the
function/logic that rewrites hardlink targets mentioned around lines 217-245).
Ensure you reference and modify is_path_safe and the hardlink-target rewrite
code so both use the same normalized-path validation routine.

sdk/runanywhere-flutter/packages/runanywhere/lib/infrastructure/download/download_service.dart (2)

364-367: ⚠️ Potential issue | 🔴 Critical

Throw on native extraction failure instead of returning the archive path.

Returning archivePath keeps the caller on the success path: it deletes the archive, updates the registry, and emits completion for an unextracted model.

Fix

       if (result != 0) {
-        _logger.error('Native extraction failed with code: $result');
-        return archivePath;
+        throw Exception(
+          'Native extraction failed with code: $result for $archivePath',
+        );
       }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-flutter/packages/runanywhere/lib/infrastructure/download/download_service.dart`
around lines 364 - 367, The code currently logs an extraction error and returns
archivePath which makes the caller proceed as if extraction succeeded; instead,
in the extraction routine where you check "if (result != 0)" (using variables
result, archivePath and logger _logger.error), throw a descriptive exception
(including result and archivePath) rather than returning archivePath so the
caller will treat it as a failure and will not delete the archive or update the
registry/emit completion for an unextracted model.

---

351-352: ⚠️ Potential issue | 🔴 Critical

Match toNativeUtf8() with the allocator you free.

toNativeUtf8() uses malloc by default, so freeing these pointers with calloc.free() is an allocator mismatch and can corrupt native memory.

Fix

-    final archivePathPtr = archivePath.toNativeUtf8();
-    final destPathPtr = destDir.toNativeUtf8();
+    final archivePathPtr = archivePath.toNativeUtf8(allocator: calloc);
+    final destPathPtr = destDir.toNativeUtf8(allocator: calloc);

Also applies to: 369-370

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-flutter/packages/runanywhere/lib/infrastructure/download/download_service.dart`
around lines 351 - 352, toNativeUtf8() defaults to malloc, but the code frees
the returned pointers with calloc.free(), causing an allocator mismatch; for
archivePathPtr and destPathPtr (and the other pointers at the same pattern
around the later block) either pass allocator: calloc to toNativeUtf8(…) or
change the frees to use malloc.free(…) so allocator used to allocate matches the
one used to free; update the calls around archivePathPtr and destPathPtr (and
the matching pointers at the later occurrence) accordingly and ensure the proper
allocator import is available.

sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_download.dart (1)

148-158: ⚠️ Potential issue | 🔴 Critical

Match the allocator used by toNativeUtf8().

These helpers allocate UTF-8 inputs with malloc (the default for toNativeUtf8()), but free them with calloc.free(). That is allocator-mismatched native memory corruption on every call path.

Fix

-      final dirPtr = extractedDir.toNativeUtf8();
+      final dirPtr = extractedDir.toNativeUtf8(allocator: calloc);
...
-      final urlPtr = url.toNativeUtf8();
+      final urlPtr = url.toNativeUtf8(allocator: calloc);
...
-      final modelIdPtr = modelId.toNativeUtf8();
-      final urlPtr = downloadUrl.toNativeUtf8();
+      final modelIdPtr = modelId.toNativeUtf8(allocator: calloc);
+      final urlPtr = downloadUrl.toNativeUtf8(allocator: calloc);

Also applies to: 176-181, 207-224

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_download.dart`
around lines 148 - 158, The UTF-8 pointer from extractedDir.toNativeUtf8() is
allocated with malloc but is being freed with calloc.free(), causing allocator
mismatch; fix by matching allocators consistently—either allocate dirPtr with
the same allocator as outPath (call extractedDir.toNativeUtf8(allocator:
calloc)) or free dirPtr with malloc.free() instead of calloc.free(); apply the
same fix to the other occurrences referenced (around the blocks for lines
~176-181 and ~207-224) so every toNativeUtf8() allocation uses the same
allocator as its corresponding free.

sdk/runanywhere-commons/src/infrastructure/download/download_orchestrator.cpp (1)

575-678: ⚠️ Potential issue | 🔴 Critical

Race condition in rac_download_orchestrate_multi() — downloads complete prematurely.

This issue was flagged in a previous review and remains unresolved. The code comments claim "sequential — each file completes before next starts" (lines 610-611), but rac_http_download is asynchronous and returns immediately after starting the download.

The current flow:

1. Loop starts HTTP download for file i
2. rac_http_download returns RAC_SUCCESS (download started, not completed)
3. Loop immediately proceeds to file i+1
4. After all downloads are started, lines 667-674 mark the task complete

This means rac_download_manager_mark_complete is called while downloads are still in flight, and failures in required files won't be detected.

To fix: After rac_http_download returns success, poll/wait for file_ctx->completed before proceeding to the next file, then check file_ctx->result to detect failures.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@sdk/runanywhere-commons/src/infrastructure/download/download_orchestrator.cpp`
around lines 575 - 678, After starting each async download with
rac_http_download (in rac_download_orchestrate_multi), wait for that file's
completion by polling the shared file_ctx->completed flag (the shared_ptr
created as file_ctx) before continuing to the next iteration; once completed
check file_ctx->result and if it is not RAC_SUCCESS treat required files
(file.is_required) as failures (set any_failed, log via
RAC_LOG_ERROR/RAC_LOG_WARNING) and break/continue accordingly, and only call
rac_download_manager_mark_complete or rac_download_manager_mark_failed after all
file results are observed; do not delete file_holder after a successful start
(the file_complete callback owns and deletes it), and update per-file progress
via rac_download_manager_update_progress based on
file_progress_start/file_progress_end before/after the wait.